Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

@matiasperrone-exo matiasperrone-exo commented Feb 5, 2026

Task:

Ref: https://app.clickup.com/t/86b7fj5wh

Endpoints affected:

Verbs Endpoint Method
GET,HEAD api/v2/users/{id} getV2

@matiasperrone-exo matiasperrone-exo self-assigned this Feb 5, 2026
@matiasperrone-exo matiasperrone-exo added the documentation Improvements or additions to documentation label Feb 5, 2026
@matiasperrone-exo matiasperrone-exo changed the base branch from feat/openapi---initial to main February 6, 2026 20:26
Copy link

@martinquiroga-exo martinquiroga-exo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matiasperrone-exo matiasperrone-exo changed the title Feature | Add OpenAPI documentation controller OAuth2UserApiController Feature | Add OpenAPI documentation controller OAuth2UserApiController v2 routes Feb 11, 2026
@matiasperrone-exo matiasperrone-exo force-pushed the feat/openapi----api-v2---oauth2userapicontroller branch from e06252a to bba5793 Compare February 11, 2026 22:38
Copy link
Contributor

@caseylocker caseylocker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. CRITICAL
    app/Swagger/Security/UsersOAuth2Schema.php uses hardcoded relative paths:
  authorizationUrl: '/oauth2/auth',
  tokenUrl: '/oauth2/token',

Per project conventions like in summit-api, these must use the L5-Swagger constants:

  authorizationUrl: L5_SWAGGER_CONST_AUTH_URL,
  tokenUrl: L5_SWAGGER_CONST_TOKEN_URL,

The .env.example already defines L5_SWAGGER_CONST_AUTH_URL and L5_SWAGGER_CONST_TOKEN_URL, and the config registers them under defaults.constants. Hardcoding defeats the purpose of environment-specific URLs.

app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.php
The getV2 route in routes/api_v2.php is wrapped in service.account middleware:

Route::get('', ['middleware' => 'service.account', 'uses' => 'OAuth2UserApiController@getV2']);

The OpenAPI attribute on getV2 has summary: 'Get a user by ID' but no description mentioning the middleware requirement. Per conventions, it should include something like:

description: 'Requires service account authentication (middleware: service.account)',

UsersOAuth2Schema.php only defines one scope:

IUserScopes::ReadAll => 'Read All Users Data',

The security scheme definition should include all scopes that will be referenced across User endpoints (not just the one used by this PR's endpoint). Looking at IUserScopes, the scheme should also include at minimum: MeRead, Write, UserGroupWrite, and Registration — any scope that future User/Group endpoints will reference.

config/l5-swagger.php Copy/Paste

'title' => 'Summit API Swagger UI',

Should be 'OpenStackID API Swagger UI' or similar. This is a copy-paste from the summit-api project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants